This is slightly changing behavior since we're now memoizing this value. Is this okay to memoize or do we need to reset it each render? I think we want to keep the old behavior and have it re-evaluate per-call to render_preview so each call has a fresh controller instance?

Added a #teardown method that clears the variable.

I mentioned it above, but not every test needs a preview controller. Can we revert this back to the previous, lazy loaded implementation to avoid extra work in test suites?

Unfortunately to support devise I don't see this being possible. We can't predict when render_preview is going to be used. The entire problem is that to support devise, setup needs to happen before render_preview is ever called. If we don't memoize, then the test may still work in most cases, but keep in mind it wont be accurate, as you're warden request environ would be a different instance than your render_preview request environ.

One possible alternative that might meet your requirements is a new concern, which sets up a test environment for devise and render_preview. That way a user is actively opting in essentially stating "I am using this pattern", and then update documentation to the concern's existence.

Honestly if it weren't for the fact that this project already has a bunch of minitests built before I started working on it, I'd just move the whole thing over to rspec and be done with it :D

Something like

module ViewComponent::TestHelpers::DevisePreviewHelper
   def self.included(base)
      base.instance_eval do
        setup do
          @request = __vc_render_preview_controller.request
        end
         
        teardown do
          @request = nil
          @vc_render_preview_controller = nil
        end
          
      # ....
   end
    
    def __vc_render_preview_controller
      @vc_render_preview_controller ||= super
    end
      
    
end

To help clarify things, are devise helpers only broken for previews or for previews + render_inline?

Just checked, it works for render_inline. I believe it is because render_preview uses an actual controller instance, causing it to pick up Warden's hooks.

This isn't correct since this is creating the request from the preview controller, not using the same request the request method uses. Can we update this to use the proper request?

I'm not sure I follow. On line 78 we have previews_controller = __vc_render_preview_controller, #request tracks to ViewComponent::Base#request which memoizes the request.

If you debug the call you can see that the request objects are indeed the same.

@request -> #<ActionDispatch::TestRequest:0x000000010c675838>
preview_controller.request -> #<ActionDispatch::TestRequest:0x000000010c675838>

These are identical. Am I missing something?

That wasn't completely clear in hindsight, sorry about that.

We shouldn't use __vc_render_preview_controller.request here because we don't need to couple the @request to the preview controller or how it's constructed. Also not every test requires a preview controller to be present either, so we should skip instantiating one except where necessary.

Actually we do. The Warden context is not correctly set up in ActionDispatch::TestRequest.create. I tested this myself with one of your earlier suggestions.

We don't always use the preview controller instance to render since users can pass their own controller class to us and render_inline uses its own controller.

What does Devise need the controller instance for?

It fails setting up the request object.

https://github.com/heartcombo/devise/blob/f6e73e5b5c8f519f4be29ac9069c6ed8a2343ce4/lib/devise/test/controller_helpers.rb#L44

@BlakeWilliams to be clear, this path is ONLY triggered if you call current_user in any of your components. Which is kindof edge case ish right now. I actually refactored so that current_user is only ever passed into components when they need it, so this PR actually wont even apply to my case anymore.

That said, I read somewhere that the long term goal of ViewComponent is that it would be used for page rendering as well, which will make this a lot less of an edge case.